-
Notifications
You must be signed in to change notification settings - Fork 8
Fixes #38862 - Handle nil host UUIDs properly #60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
547e515 to
23fc6f2
Compare
23fc6f2 to
0e4003d
Compare
ianballou
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking fine, just some test questions.
| ] | ||
|
|
||
| put '/update_hosts', { 'hosts' => hosts } | ||
| assert last_response.ok? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test doesn't verify that no hosts were added to the DB.
| .map { |repo| repo['repository'] } | ||
| end | ||
|
|
||
| def update_host_repositories(uuid, repositories) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a test with nil UUIDs in test/container_gateway_backend_test.rb ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used claude and it added some tests that cover some blank uuid cases.. 🤖
0e4003d to
a0bae61
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Tested syncing & pull content from the smart proxy via a registered cert host just to be safe.
Add some nil UUID handling. This will work independently of the Katello changes : Katello/katello#11569 which prevents katello from sending this nil values by switching to use subscription UUID as the host identifier.
To test,
Register a host to a Smart proxy.
In console, empty out the content facet UUID.
Sync the smart proxy.
The sync should succeed. Try normal workflows like 2 registered hosts with one nil UUID and another with UUID etc.